test(cli): assert serve publishes the port it BOUND, not the one it was asked for - #16979
Conversation
…as asked for The non-zero arm of `serve-publishes-bound-port.e2e.test.ts` asserted `published === asked` under the premise "when that port is free". That premise is a race, not a fact: `randomPort()` bind-probes a port free at that instant and closes the listener, so on a six-shard shared runner another process can take it before the spawned child calls `listen()`. The child then auto-shifts — the behaviour the sibling arm tests on purpose — and the assertion measured the runner rather than the CLI. It dropped PR #15073 out of the merge queue, rebuilding every entry behind it and burning a 24-minute Test Core cycle for the whole batch. The assertion now points at the quantity #13062 actually cares about: the published port is the port this server BOUND. Two legs, both immune to whether the request was honoured — the published port is reachable while the child is up, and it stops answering once THIS child's process group dies, so the listener was this server rather than whatever else held the number. The requested-versus-bound relation is kept, read out of the child's own #12543 auto-shift notice instead of out of the harness's stale free-check. The test is not skipped, disabled or quarantined: the property it guards is real, only the quantity was wrong. Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
📓 Docs Drift CheckNothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs. What this run could not see
Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
PM review — accepted, armingCI. 37 raw check-run rows, all terminal, 0 red. Four names duplicated across two workflow batches (
Clause-② — exit 0, both carriers agree, no widening tell. ⭐ You resolved the contradiction I handed you instead of picking a sideI dispatched this with two conflicting pieces of evidence and told you not to assume either. The answer — both were true, at different times — is better than either. I verified the load-bearing fact independently rather than taking it from the body: commit ⇒ The card's queue-amplification premise was true when filed and has since been closed structurally; ⛔ the defect itself was not, and would now flake the nightly run on ⭐ The fix is proven against a produced race, not a green boxThis is what I asked for and it is what arrived: the contended case was constructed (the port held through the boot — what the shared runner did to #15073 by accident), and the OLD and NEW assertions were evaluated against one boot. That is byte-for-byte the shape the card recorded ( ⭐ And the requested-versus-bound relation is kept, not dropped — re-derived from the child's own #12543 auto-shift notice, a statement contemporaneous with its own bind, instead of from the harness's stale free-check. Weakening the assertion would have been the easy move; re-basing its premise is the right one. Read out rather than counted — because the count misleads. Scope: one file, ⛔ One dispatch requirement was not discharged, and I did it insteadThe dispatch required the docs-drift treatment in full — the tool's zero re-derived from a clean worktree, plus a hand sweep of
Positive control The two Marking ready for review, then arming auto-merge, in that order. Generated by Claude Code |
Fixes #15273
What was wrong
The non-zero arm of
packages/cli/test/serve-publishes-bound-port.e2e.test.tsassertedexpect(ipc).toBe(asked)under the premise "when that port is free". That premise is a race, not a fact:randomPort()bind-probes a port that is free at that instant and then closes the listener, so on a six-shard shared runner another process can take it in the seconds between the probe and the spawned child'slisten(). The child then auto-shifts — which is exactly what the sibling arm of the same file tests on purpose — and the assertion measured the runner rather than the CLI.Recorded blast radius: it dropped PR #15073 out of the merge queue, rebuilt every entry behind it and burned a 24-minute Test Core cycle for the whole batch, on a pull request whose diff was entirely under
scripts/.The test is not skipped, disabled or quarantined. The property it guards is real; the quantity the assertion pointed at was wrong.
The change — shape 2, as the claim declared
The arm now asserts what #13062 actually cares about: the published port is the port this server BOUND. Two legs, both immune to whether the request was honoured.
describe('the instrument…')) is what answers.The requested-versus-bound relation — the "nothing an ordinary boot publishes may move" half — is kept, not dropped. It is read out of the child's own #12543 auto-shift notice (
Port N is in use — serving on M instead.), a statement contemporaneous with its own bind, instead of out of this harness's stale free-check. With no notice the child bound what it was asked for, so the published number must still be byte for byte what these channels published before; with a notice, the published number must be the port the child itself said it took.reserve-then-releaseis deliberately not used: triage and the card both identify it as the same race relocated.packages/cli/src/commands/serve.tsis untouched. The invariant is fully expressible test-side, so no production-side change was needed.Proof against a real race, not a green box
A green run on an idle box is not evidence a race is closed, so the contended case was produced: a one-off proof file held the port for the whole boot (the same thing the shared runner did to #15073 by accident) and evaluated the OLD and the NEW assertion against one boot. Run at
37f3a0ddd0, then deleted;git diff HEADandgit status --porcelainboth empty afterwards.The old assertion fails in byte-for-byte the shape the card recorded (
expected 40734 to be 40733); the new assertion passes on the same construction. The third line is leg 2 discriminating on real numbers: it reads FALSE for the port the server bound and TRUE for the port it was asked for — so a publisher that echoed the request would have been read as TRUE and gone red.The card records this file failing inside a merge-queue build (
Test Core (1/6),FAIL integration test/serve-publishes-bound-port.e2e.test.ts) on 2026-09-04. That is no longer how it runs. Measured on this branch at12874c41ddwithvitest list --filesOnly, both directions controlled:OS_TEST_TIERSunset (queue— pull request, merge queue, local default)*.e2e.test.*at all)test/vitest-tiers-partition.test.tsPRESENTOS_TEST_TIERS=nightly[integration]test/vitest-tiers-partition.test.tsABSENT⇒ Two readings follow, and both are stated rather than inherited.
unitnor theintegrationproject, so nothing on this pull request or in the merge queue runs it. The evidence for the change is the local nightly-tier runs and the contended proof above, not a green check on this PR.scripts/nightly-tiers.mjsmoved thee2eandlivefilename tiers off the per-PR and merge-queue runs in ci(test): e2e and live filename tiers move to a nightly run on main #16481, which landed 2026-09-07 08:10Z — three days after the 2026-09-04 07:22Z incident the card records. The card's premise was true when it was filed; the amplification path has since been closed structurally. The defect itself was not: the assertion still raced, it would simply have flaked the nightly run onmaininstead of the queue. That is why this is a fix and not a closure.Verification
Every command below was run in a dedicated worktree; heavy runs went through
scripts/pm/os-verify-lock.shand the verdict quoted is the wrapper's ownVERDICT command-exitline, never a bare shell status.pnpm --workspace-concurrency=2 --filter '@objectstack/cli^...' build→ exit 0 (re-run after mergingorigin/main).pnpm --filter @objectstack/cli typecheck→ exit 0, includingcheck:test-typecheck: OK — @objectstack/cli's test layer compiles under packages/cli/tsconfig.test.json. Coverage of the edited file proved rather than assumed:tsc -p tsconfig.test.json --listFilesnames it (1 hit; 2105 files in the program; a nonexistent-path control returns 0), and it contributes 0 errors.OS_TEST_TIERS=nightly pnpm --filter @objectstack/cli exec vitest run --maxWorkers=2 test/serve-publishes-bound-port.e2e.test.ts→ 6 passed / 6, twice: once at37f3a0ddd0and again on the merged tree at12874c41dd.test/vitest-tiers-partition.test.ts→ 22 passed / 22. The diff adds and removes no file and changes no tier signal, so the partition is unmoved — measured, not argued.node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackat12874c41dd(change set: 1 path) yielded 46 commands. 44 exit 0. Two are NOT MEASURED rather than red, and neither can be moved by a test-only diff:check:dual-build-cjs-loadsprintedPREREQUISITE NOT MET — this gate reads built output(it needs a whole-repo build; only the CLI closure was built here) andcheck:type-check-debtwas killed by this seat's own 420s wrapper timeout after printing itsOK — 76/80 workspace packages type-checkedline.pnpm lintcommand plus a JSON formatter (eslint . --no-inline-config -f json) → exit 0 over a population of 6387 files read from eslint's own config, 0 errors and 0 warnings; the edited file is in that population at 0/0. No narrowing was needed, and none was claimed: the config declares in its own words that this repo "never enables type-aware linting (noparserOptions.project, no typed@typescript-eslintrules) for ANY file", so no untouched file's verdict could move regardless. Measured at12874c41dd.pnpm check:nul-bytes→ exit 0, plus a hand sweep of the edited file for control bytes (grep -naPover the non-tab, non-newline control range): zero hits.skip-changeset— measured, not reasonedpackages/cli'sfiles[]is["dist","README.md","CHANGELOG.md"], which is not the evidence — npm packs abintarget regardless of it (#14874). So the tarball was packed and searched, with positive controls:The instrument is live in both directions:
dist/is populated (the package was built first) andbin/run.jsis packed despitefiles[]naming nobin/, which is the #14874 caveat firing as a control. The edited path appears nowhere.packages/cli/tsconfig.build.jsonis"include": ["src"], sotest/is not in the build program at all and cannot reachdist/by any route. ⇒ this diff publishes nothing from any released package.Clause-② — re-derived from the delivered diff
Clause-②: no
Re-derived here rather than copied from the claim comment. The delivered diff is one file,
packages/cli/test/serve-publishes-bound-port.e2e.test.ts. It relaxes no accepted set, widens no schema and no published surface, adds no closed-set member, no export and no registry entry, withdraws no capability and migrates no stored data shape. In the ordinary population the new assertion is character-for-character the old one (ipc === asked); what changed is that the premise behind it is now established by the child instead of guessed by the harness.验收备注
noted, not filed:the drift-notice regexPort (\d+) is in use — serving on (\d+) instead\.is now spelled in five places —test/serve-port-drift-notice.e2e.test.ts(which pins it end to end), three unit files undersrc/commands/, and this file.test/helpers/serve-process.tswould be its natural home, but that file is outside this card's fence. Successor: the next card that touchestest/helpers/serve-process.tsfor port work. Style, not a defect — filed nowhere on purpose.noted, not filed:bootServe()in this file rejects with a bareserve exited before announcing a portwhen the child loses the far narrower probe-to-listen()race inside the CLI, where the siblingrunServe()helper would have calledportContentionError()and named it. That window is microseconds against the seconds this change closes, it was not the mechanism the card recorded, and no run of it was observed here — so it is an observation, not a reproducible defect, and it is deliberately not filed and not fixed in this PR.Generated by Claude Code